Skip to content

fix: calculate run memory min/max memory clamping#986

Merged
danpoletaev merged 3 commits into
masterfrom
fix/calculate-run-memory-min-max
Jan 6, 2026
Merged

fix: calculate run memory min/max memory clamping#986
danpoletaev merged 3 commits into
masterfrom
fix/calculate-run-memory-min-max

Conversation

@danpoletaev
Copy link
Copy Markdown
Contributor

This PR updates the dynamic run memory calculation (added in #980) to respect the memory limits set in actor.json.

The final memory value is now clamped between minMemoryMbytes and maxMemoryMbytes.

In case user provides defaultMemoryMbytes via flag, we're not checking for min/max memory from config, because it feels confusing.

@danpoletaev danpoletaev requested a review from tobice January 5, 2026 12:37
@danpoletaev danpoletaev self-assigned this Jan 5, 2026
@danpoletaev danpoletaev added the adhoc Ad-hoc unplanned task added during the sprint. label Jan 5, 2026
@github-actions github-actions Bot added t-core-services Issues with this label are in the ownership of the core services team. tested Temporary label used only programatically for some analytics. labels Jan 5, 2026
Comment thread src/commands/actor/calculate-memory.ts Outdated
// If not provided via flag, try to load from actor.json
if (!memoryExpression) {
memoryExpression = await this.getExpressionFromConfig();
const {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(nit) You should be able to do:

let memoryExpression: string | undefined = defaultMemoryMbytes;
let minMemoryMbytes = 0;
let maxMemoryMbytes = Infinity;

if (!memoryExpression) {
     ({ defaultMemoryMbytes: memoryExpression, minMemoryMbytes, maxMemoryMbytes } = await this.getExpressionFromConfig());
 }

I'm not sure about the exact syntax but it should be possible to destruct into variables defined via let.

Comment thread src/commands/actor/calculate-memory.ts Outdated
@@ -63,10 +69,16 @@ export class ActorCalculateMemoryCommand extends ApifyCommand<typeof ActorCalcul
const { input, defaultMemoryMbytes, ...runOptions } = this.flags;

let memoryExpression: string | undefined = defaultMemoryMbytes;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: maybe extract all this logic inside a function in the class and return it for the run method to consume instead

Comment thread src/commands/actor/calculate-memory.ts Outdated
return localConfig?.defaultMemoryMbytes?.toString();
return {
defaultMemoryMbytes: localConfig?.defaultMemoryMbytes?.toString(),
minMemoryMbytes: localConfig?.minMemoryMbytes as number,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cast is a lie, its number | undefined

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally we need to define actor.json type in future, that will help in such cases

@danpoletaev danpoletaev requested a review from vladfrangu January 6, 2026 08:29
@danpoletaev danpoletaev merged commit 0101cb1 into master Jan 6, 2026
22 checks passed
@danpoletaev danpoletaev deleted the fix/calculate-run-memory-min-max branch January 6, 2026 12:32
@danpoletaev danpoletaev added the validated Issues that are resolved and their solutions fulfill the acceptance criteria. label Jan 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

adhoc Ad-hoc unplanned task added during the sprint. t-core-services Issues with this label are in the ownership of the core services team. tested Temporary label used only programatically for some analytics. validated Issues that are resolved and their solutions fulfill the acceptance criteria.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants